Skip to content

Dev/t feadeaga error changes#62

Open
FeyiAdeaga wants to merge 42 commits into
mainfrom
dev/t-feadeaga_error_changes
Open

Dev/t feadeaga error changes#62
FeyiAdeaga wants to merge 42 commits into
mainfrom
dev/t-feadeaga_error_changes

Conversation

@FeyiAdeaga

Copy link
Copy Markdown

No description provided.

@alsanmsft

Copy link
Copy Markdown
Collaborator

might be good to link spec or provide overview of the context in the description of the PR

Comment thread internal/exec/exec.go
if ok {
if status, ok := exitErr.Sys().(syscall.WaitStatus); ok {
exitCode = status.ExitStatus()
commandExitCode := exitCode

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

question: why do we differentiate between exitCode and commandExitCode here? What is the meaningful difference?

Comment thread pkg/download/save.go
f, err := os.OpenFile(dst, os.O_WRONLY|os.O_TRUNC|os.O_CREATE, mode)
if err != nil {
return 0, errors.Wrapf(err, "failed to open file for writing: %s", dst)
return 0, vmextension.NewErrorWithClarificationPtr(constants.FileDownload_OpenFileForWriteFailure, errors.Wrapf(err, "failed to open file for writing: %s", dst))

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It may be worth considering differentiating between when a file fails to be open because the path does not exist (since this func doesn't create the directory of destination) versus when a file fails to open for writing for some other reason.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants